feat: detach foreground tasks to background#821
Conversation
🦋 Changeset detectedLatest commit: e480603 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0c9dd763e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| outcome: Outcome, | ||
| ): Promise<Outcome> { | ||
| if (timeoutMs === undefined || timeoutMs <= 0) return NEVER; | ||
| return sleep(timeoutMs).then(() => outcome); |
There was a problem hiding this comment.
When a task finishes before its deadline, this sleep() promise is the losing branch of Promise.race (for example in runTaskLifecycle() and wait()), but the underlying timeout is neither cleared nor unref()ed. A quick foreground Bash call now leaves the default 60s timer alive, and a quick background Bash call leaves the default 10-minute timer alive, so short-lived SDK/CLI processes can hang until those deadlines even though the task is already terminal; use a cancelable/unref'd timeout or clear it in the race cleanup.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3995e1e90
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| }; | ||
| stream.once('end', done); | ||
| stream.once('close', done); | ||
| stream.once('error', done); |
There was a problem hiding this comment.
When stdout or stderr emits an error while the process itself exits successfully (for example an SSH/channel read failure), this handler resolves the stream promise and ProcessBackgroundTask.start() can report the task as completed with partial or missing output. The foreground Bash path now relies on this helper too, so users can see a successful command even though output capture failed; reject on non-abort stream errors and only suppress the expected errors from cancellation/stream destruction.
Useful? React with 👍 / 👎.
| const requestAbort = (): void => { | ||
| this.abort?.(); | ||
| this.abortController.abort(); |
There was a problem hiding this comment.
For a foreground subagent cancelled by the user after registration, the manager aborts the sink signal but this callback aborts the subagent controller with no reason, so SessionSubagentHost and child tool calls see a generic AbortError instead of the original user-cancellation reason. That regresses the existing isUserCancellation(signal.reason) handling used to report deliberate interrupts and fire interrupt hooks; forward the sink/original abort reason when aborting the subagent controller.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64ad9611a2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Foreground Bash commands (run_in_background omitted) were registered with the BackgroundManager as detached:false, which immediately persisted task metadata and streamed the full stdout/stderr to tasks/<id>/output.log. Those terminal detached:false tasks are filtered out of every listing, so ordinary foreground commands left undiscoverable full-output logs in the session directory — a disk-growth regression for large or frequent commands. Foreground tasks now keep their output in memory and touch disk only once they actually detach (or spill past a 1 MiB in-memory bound). On detach the buffered pre-detach output is flushed first so output.log stays the complete, in-order record. A foreground task that finishes without detaching or spilling writes nothing to disk. Spilled/detached records are persisted and kept as before — not deleted.
…etach # Conflicts: # packages/agent-core/src/rpc/core-impl.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e480603aff
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (builder.nChars > 0) builder.write('\n'); | ||
| builder.write( | ||
| `task_id: ${taskId}\n` + |
There was a problem hiding this comment.
When a foreground command has already filled ToolResultBuilder's 50k output cap before it is detached, these writes are ignored or only partially appended, so the tool result can say the task moved to the background without including the task_id needed for TaskOutput/TaskStop in prompt or SDK clients. This can happen with long-running noisy commands that the user detaches after seeing a lot of output; reserve space for the metadata or write it with a fresh builder instead of appending it after capped command output.
Useful? React with 👍 / 👎.
| readonly taskId: string; | ||
| readonly description: string; | ||
| readonly status: AgentCoreBackgroundTaskStatus; | ||
| readonly detached?: boolean; |
There was a problem hiding this comment.
Adding detached only to the TypeScript interface leaves the exported Zod schema out of sync: backgroundTaskInfoBaseSchema still omits this field, and Zod strips unknown object keys by default when callers parse background.task.started/terminated events through eventSchema. Protocol consumers that validate events will therefore lose detached: false and cannot distinguish foreground tasks from detached background tasks despite the new protocol field.
Useful? React with 👍 / 👎.
Related Issue
No linked issue. This addresses a maintainer-requested core/API capability for moving foreground shell and subagent tasks into the background.
Problem
Foreground Bash and subagent tasks are managed as blocking tool calls. If a model starts one without
run_in_background, there was no core RPC path for a client to detach that already-running foreground task and continue treating it as a background task.What changed
BackgroundManager, so tasks can start foreground and later detach without switching lifecycle owners.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.Verification:
pnpm --filter @moonshot-ai/agent-core typecheckpnpm --filter @moonshot-ai/agent-core exec vitest run test/tools/background/task-tools.test.tspnpm --filter @moonshot-ai/agent-core exec vitest run test/tools/bash.test.ts test/tools/shell-quoting.test.ts test/tools/shell-cancel.test.tspnpm --filter @moonshot-ai/agent-core exec vitest run test/agent/background/agent-timeout.test.ts test/agent/background/manager.test.ts test/agent/background/rpc-events.test.ts test/tools/background/task-tools.test.ts test/tools/agent.test.ts test/tools/builtin-current.test.ts test/session/lifecycle-hooks.test.ts test/agent/bg-idle-notification-repro.test.ts test/agent/background/ids.test.ts test/tools/bash.test.ts test/tools/bash-env.test.ts test/tools/shell-cancel.test.ts test/tools/shell-quoting.test.tsgit diff HEAD --check